Use lightning::util::anchor_channel_reserves #674
Use lightning::util::anchor_channel_reserves #674Camillarhi wants to merge 1 commit intolightningdevkit:mainfrom
lightning::util::anchor_channel_reserves #674Conversation
|
I've assigned @tnull as a reviewer! |
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 4th Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 5th Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 6th Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 7th Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 8th Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 9th Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 10th Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 11th Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 12th Reminder Hey @tnull! This PR has been waiting for your review. |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Excuse the delay here, this looks already pretty good, but I have a few comments. This unfortunately also needs a rebase by now - sorry!
Given how high the current dynamic estimation result currently is I do wonder if we need to give other defaults or if we even can realistically switch to it.
| } | ||
|
|
||
| /// Returns the configured anchor channel reserve per channel in satoshis. | ||
| pub fn get_anchor_reserve_per_channel() -> u64 { |
There was a problem hiding this comment.
I don't think we should make this public, esp. given the only other call site is our tests.
| let addr_b = node_b.onchain_payment().new_address().unwrap(); | ||
|
|
||
| let premine_amount_sat = if expect_anchor_channel { 2_125_000 } else { 2_100_000 }; | ||
| let anchor_reserve = if expect_anchor_channel { get_anchor_reserve_per_channel() } else { 0 }; |
There was a problem hiding this comment.
Rather than calling this to get the dynamic value, can we still keep a fixated value here that would also allow us to check if the method works as expected and detect if/when something changes.
| /// **Note:** Depending on the fee market at the time of closure, this reserve amount might or | ||
| /// might not suffice to successfully spend the Anchor output and have the HTLC transactions | ||
| /// confirmed on-chain, i.e., you may want to adjust this value accordingly. | ||
| pub per_channel_reserve_sats: u64, |
There was a problem hiding this comment.
I think we might want to keep this as an override and make it an Option<u64> so that user can still set a specific value if they want. But probably the default should be None, i.e., dynamic estimation.
| /// Returns the configured anchor channel reserve per channel in satoshis. | ||
| pub fn get_anchor_reserve_per_channel() -> u64 { | ||
| let context = AnchorChannelReserveContext::default(); | ||
| get_reserve_per_channel(&context).to_sat() |
There was a problem hiding this comment.
Hmm, it seems the current defaults would result in a channel reserve of 336513sats (~$300) per channel. This seems very conservative, and much too high for the typical use case we currently expect in LDK Node. This makes me wonder if we either need to use different defaults, or not opt for dynamic estimation right now. Will discuss this with a few other devs and get back to this.
There was a problem hiding this comment.
Just want to check if there is an update on this after discussing with the devs
…serve estimation Replaced flat fee-based reserve logic with estimation using `get_reserve_per_channel`, following changes introduced in lightningdevkit/rust-lightning#3487.
Camillarhi
force-pushed
the
update-anchor-channel-reserve-estimation
branch
from
e7029bf to
9daa985
Compare
3 participants
This update replaces the flat fee requirement for anchor channel reserves with a more accurate estimation using the new
lightning::util::anchor_channel_reservesutility.Closes #668